@nodejs/modules-active-members I couldn't find a sane way to refactor the getFormat hook in this PR. We should figure out a way to make it not do 2 requests.

Interesting, would there be a way to statically know what an app uses? Right now Sqreen parses the content of the diverse node_modules and we consider it as "good enough" as a source of truth.

@vdeturckheim due to import() using an expression, not unless there is no usage without a literal. A policy file could enforce a static codebase though.

@bmeck true, I often kind of think that import() is somehow the eval() of modules and could have a flag to be disabled but this would be a totally different topic.
I like the policy idea, it sounds pretty elegant.

I couldn't find a sane way to refactor the getFormat hook in this PR. We should figure out a way to make it not do 2 requests.

This may a new motivation to revive #35524. I think that should fix the issue (but it's potentially disruptive).

I think this is really cool and it would be cool if this loader (optionally?) cached resources (according to HTTP headers, like browsers do). I think this is what you mean by "Offline cache" :]

@jkrems an alternative is to have a resource cache in our default loader rather than doing everything at time of request, we can drop the resource once it ref counts down to 0 or some other metric.

@mcollina I'm not entirely opposed to flagging this (such as requiring it to be in a policy file listing), but if this leads to users consistently enabling them by default it would be good to understand why HTTPS (not HTTP) is insecure from your perspective so we can mitigate any issues.

@mcollina... as this is still a draft and being actively discussed, I'm not sure the "-1"/Request Changes is helpful yet.

@bmeck ... the security concerns on this really come down to the general risk of running untrusted code downloaded over arbitrary internet connections. For instance, I could fairly easily write a script that returns one bit of javascript during development but replaces that with a malicious script when accessed from a production server, without the developer ever knowing that there's been a change. Putting this behind a flag for now while we figure out the threat model and various mitigations definitely makes sense and would be right thing to do.

@jasnell I've stated a security model document, though due to the lack of safety in using core's HTTPS a lot will inevitably be labeled as out of scope of this feature and must be secured by other means like policies. I would note that event-stream did the same kind of replacement workflow on files locally and do not consider this attack vector novel by adding HTTPS. Node's mitigation for that attack surface is to use a policy and the same would be true here.

It is insecure because there is no signature of the original file. Anybody that can take control of a domain name can take control of a server.

As a community we had some significant issues regarding security, from left-pad to event-stream. Adding this functionality would make us 10x more vulnerable to these kind of a attack because there would be no central entity that could intervene.

@mcollina can you clarify how policies aren't that intervention/mitigation?

@mcollina... as this is still a draft and being actively discussed, I'm not sure the "-1"/Request Changes is helpful yet.

The plan at the top highlight the fact that this will be enabled by default and it needs consensus to make users disable this. I do not agree with that approach, hence my comment. I'm happy to change my mind if it can be made safe to do so - however I do not think it is possible at all to make this safe.

@bmeck .. for discussion.. from your checklist:

Can be disabled - NEEDS CONSENSUS

Initially I would mark this explicitly opt-in. Assuming we flip that default later, there should definitely be a way to explicitly opt-out on the process level.

Offline cache - NEEDS CONSENSUS
established file location / configuration
ability to eject/override cache features agreed upon
cache semantics agreed upon

Definitely a much larger discussion and one that we need to have with the package manager folks at the table. Not only would we need to figure out the caching semantics, we need to figure out the semantics around what happens if a require('https...') script does a require('some-local-script'). What if those things end up being different versions? I think what we need to do is take a moment to draw up a list of What Ifs and see if we can easily answer those.

Integrity checks - WIP

I'm going to sound like a broken record but as I've been saying for years now we 100% need this. Adding https imports would only make that need more pronounced.

policies should cover? needs tests for protocol => domain => / separator => URL
certificate authorities
Cookies - NEEDS CONSENSUS
should this even be allowed / same-origin meaning, agreed upon

Blocklist / Allow list of origins at the very least.
Same-origin is going to be hard. I can imagine a case where require('https...') effectively becomes its own realm, where any of the code running within is isolated in it's own context, is only permitted to use either require('{built-in}') or require('https...') within the same origin or following origin policies, etc.

Cookies are a much more difficult matter. My knee jerk reaction is that we should not support cookies.

Redirects - WONTFIX / banned
redirecting across protocols is... insane
they work significantly different than files, making dumping to disk break in odd ways
they make import.meta.url mean something very different by no longer having a 1-1 mapping for Web compat
meaning for policies is very complicated (need to add aliasing config to policies if supported)

This I'm less convinced about and need to think through more. My knee jerk reaction is that not supporting redirects in some way is very anti-web and therefore a bad thing but that's the Standards Wonk side of me speaking. Will stew on this one.

MIME - FIXME
MIME parser is... still blocked? I'm not willing to go through the consensus/ anguish to move that forward; too much blood lost already to the great consensus gods and it feels like I will cry (not in a joking manner) if I try to land it again.

I think I may be able to take this over for you but it would realistically be a 2021 project.

Selective HTTP support - NEEDS CONSENSUS

I think we can safely just say no to http modules.

Debugging HTTPS on a local machine is painful, should have some way to do local dev.

This mechanism should allow for locally aliasing of https modules anyway, making it so that even if require('https...') is used, if there is a local alias established for it, the local one is used. That should address at least the immediate issue. However, the point here still stands: we'll need to provide observability into what is happening during load. We do have keylog support already built in to core. We'll want to make sure we have command line options to allow keylogging for Node.js' own module loader connections.

@mcollina we need to understand why you consider it unsafe given current features and mitigations for these workflows that already exist. I don't think a blanket statement that it is unsafe and implication that it cannot be safe is helpful.

@bmeck ... I'm going to take a bit more time to dig through your write ups and code and think it through before responding much more as I don't want to just churn the conversation here. Hopefully others will do the same. I'll definitely take on the MIME module work tho if that would make things easier for you.

The attack vector is the same of left-pad and event-stream. There are plenty of write ups around on what happened. The mitigations that npm put in place are not feasible without a central/federated authority or strong cryptography.

I don't have time right now for a long write-up, I'll try to get back as soon as I can.

What would make this safe is giving authors a way to cryptographically sign the published files, and let developers validate those files against said keys that are fetched off-band.

Overall I would recommend developing this as a module/loader and then propose its addition to Node.js.

What would make this safe is giving authors a way to cryptographically sign the published files, and let developers validate those files against said keys that are fetched off-band.

I don't understand this. I am vehemently against code signing on the developer if it is done improperly and have met over the years with a few certificate authorities about the complexity of doing this. This reduces down to the same workflow as TLS where you pull the keys off the trusted CA which is out of band by nature then verify. What advantage do we get here? Even if they upload a personal key such as using PGP the revocation mechanism and hijacking or repurposing under a different key are all possible.

Overall I would recommend developing this as a module/loader and then propose its addition to Node.js.

What is "this"? I'd note that a variety of the mitigations are not possible in user land loaders (such as policy integration) and loader have repeatedly been stalled out by Node's process so I doubt they will move forward any time soon. I spent a lot of time on loaders and trying to move them forward and don't think they are worth the push back from the consensus model we use in Node core.

Hey all, so I brought up some points recently on Twitter related to this proposal, so I thought I'd re-articulate them here. Keep in mind I've never contributed to Node.js, this is just my opinion as a long-time user and a contributor to Deno.

TL;DR:

Importing from npm isn't inherently more secure than importing from a raw URL, so let's work to make the whole ecosystem more secure, regardless of how packages are imported.

URL Imports Aren't Inherently Less Secure

First I want to address the biggest concern that always comes up when talking about URL imports: "It's insecure." The argument is usually summed up similarly to what @jasnell said earlier in this thread: "I could fairly easily write a script that returns one bit of javascript during development but replaces that with a malicious script when accessed from a production server, [...]"

It's true, but it implies that the current way of importing is somehow more secure, and that it is more secure by virtue of not explicitely using a URL. However, packages hosted on npm are vulnerable to this problem as well. Not even a month ago, there was another report of malicious code being distributed in an npm package.

We've become accustomed to assume that central registries are somehow more secure, but it's simply not the case. Packages are still fetched from a remote source and malicious code can still make its way into npm. Once we accept that reality, URL imports become much less scary (or central registries become much more scary, depending on how you look at it).

The concerns raised here are all very valid; We just have to remember that they are issues that also affect the current import model, and so I don't it's fair to block this proposal on the grounds that it's "less secure".

I would even argue that URL imports have the potential to be more secure than traditional imports through npm, because it leverages two important aspects of the web: Authority and Visibility. It gives end-users the possibility to verify that they are imported from a trusted domain name and that that domain name has a valid SSL certificates that verifies that it is indeed that domain. It also allows to verify the whois information associated with that domain, potentially being much more explicit about when a domain or a package changes ownership. (As an aside: wouldn't you rather import express directly from https://expressjs.org than https://npmjs.org ?)

Those are all things that end users can implement themselves and include in their CI/CD pipelines and compliance tools. They've been the backbone of the internet for a long time. Browsers trust them. Users trust them. Why couldn't we?

Feedback On The Concerns Above

The other part of @jasnell comment - "[...] without the developer ever knowing that there's been a change." is very important; I think if URL imports are to go forward, they should be restricted to tagged versions, no "latest" allowed. deno.land/x uses the @ symbol to identify versions. While we don't support semver type versioning like package.json does, it's probably possible to implement in order to support the same version limiting already available in Node.

Local caching and interop with require("some-local-package") are also two good points. Now, this might just be me being naive about the complexity of the node_modules system, but we could simply cache url-imported modules in node_modules: It would make the old style of imports compatible with url imports and solve the issue of the local cache. Sure it wouldn't be compliant with http cache headers, but then again, node_modules already isn't and that's never been a problem in the past. In fact, I think there's a larger argument to be made here that being compliant with http cache headers don't make much sense on the server side.

Similarly, I agree with @jasnell with regards to the cookies support; I don't think it makes much sense here.

I'm less certain about redirects, they can be helpful for resolving versions when using tags like ^1.6 or something. Maybe restrict them to the same origin?

I don't see a reason to not include an allow/block function for specific origins, though I think we should also add a recommendation in the docs for operators to include those in firewall rules (hey if we're gonna take advantage of web mechanisms, let's go all the way!)

Now for a big one: integrity checks. Honestly, I don't know how much we can do there. Ultimately we can include checksum verification to make sure that the content the user receives is the same one the server said it was sending and that it wasn't tempered with. Beyond that, I don't there's much else that can be done. As I said earlier, it's already possible for legitimate users to upload malicious code to a legitimate platform. The best anyone can do here is certify that the content wasn't messed with by a man-in-the-middle.

I'm less certain about redirects, they can be helpful for resolving versions when using tags like ^1.6 or something. Maybe restrict them to the same origin?

They can be but the way the web is specified leads to issues if you do. Let us imagine:

1. a function resolve = import.meta.resolve that resolves URLs for importing purposes.
2. a url /react that redirects to /react/1.6

On the web await import(resolve('/react')) !== await import('/react') because one is cached at /react and the other at /react/1.6. This problem is so bad in ESM when it was possible to cause this via having both ESM and CJS that we have a whole docs section on it for Node https://nodejs.org/dist/latest/docs/api/packages.html#packages_dual_package_hazard

Per integrity checks that many people keep bringing up please read https://nodejs.org/dist/latest/docs/api/policy.html as they do already cover most of these concerns. However, like @wperron is bringing up, they assert integrity not if the resource is hostile or not.

how about gopher imports?

I think https imports are not great for security. I also think npm is not great for security. I'm fine with node supporting both. If one wants to argue that we should only be adding "secure" module systems moving forward, it would probably be a better use of time to discuss some sort of decentralised immutable protocol or something.

Restricting https imports to some top-level --unsecure flag that requires policy to be populated to disable might make sense. It would be nice to see what the overall workflow looks like there that it doesn't end up similar to Deno in making adding --unstable rote.

One ecosystem concern I have here is npm packages starting to ship imports to URLs in them. This risks an npm ecosystem that becomes dynamically attackable from these https vectors. It would be nice to explicitly disable support for that somehow in the resolution mechanism. Eg turning this feature off in the module graph once file-based package resolutions have been hit.

It would be nice to explicitly disable support for that somehow in the resolution mechanism. Eg turning this feature off in the module graph once file-based package resolutions have been hit.

Can you clarify this idea?

Landed in ceadb47

Thanks @Trott ! Congrats @JakobJingleheimer !

I'm incredibly excited for this, thanks for adding it! Any word on what will be the first version of Node this will land in and in which I can expect the CLI flag to work? Thanks!

Would there be a good way to check an https import resource with a checksum?

@mrozbarry you can use policies for integrity checks https://nodejs.org/dist/latest-v17.x/docs/api/policy.html

Unrelated to my previous comment, what if domains could be allowed/white-listed in a package.json? That could at least prevent imports from importing from some unknown domain. Maybe on first run, before cache, node could prompt "Do you trust domain.tld? [yN]", which would just add some sort of "allowedDomains": ["domain.tld"] to package.json. I think that could eliminate a collection of hijacking problems.

@mrozbarry that is possible in a policy

@mrozbarry you can use policies for integrity checks https://nodejs.org/dist/latest-v17.x/docs/api/policy.html

It would be great if node could automatically handle generating these integrity checksums for new https resources if they don't exist already. I'm going to assume that most js scripts in the wild don't have their own checksums.

Also, is there a good standard in place for servers that don't want someone just importing their scripts? I'm assuming CORS won't really cover that, considering know doesn't necessarily have to have a concept of it's own server.

@mrozbarry please open new issues, this is a merged PR

I suggest locking as resolved.